-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dev: Add Most F Rules to Worker #496
Conversation
@@ -293,7 +293,10 @@ def _row(title, c1, c2, plus="+", minus="-", neutral=" "): | |||
spacer = ["=" * row_w] | |||
|
|||
title = "@@%s@@" % "{text:{fill}{align}{width}}".format( | |||
text="Coverage Diff", fill=" ", align="^", width=row_w - 4, strip=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strip is an unused Param here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was already fixed after rebasing 🤷
@@ -161,32 +161,6 @@ def test_flake_consecutive_differing_outcomes_no_main_branch_specified(dbsession | |||
assert flaky_tests[test_id] == {FlakeSymptomType.CONSECUTIVE_DIFF_OUTCOMES} | |||
|
|||
|
|||
def test_flake_consecutive_differing_outcomes_no_main_branch_specified(dbsession): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joseph-sentry This test looked to be a copy of the one above it, let me know if we actually wanted to keep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's an exact copy, it's probably a slip up and can be deleted, but if they differ a bit, then it's probably a name mixup and we should rename and keep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it was an exact copy :o
@@ -68,7 +68,7 @@ def test_all_report_rows(self, mock_handle_single_row, dbsession): | |||
def mock_handle_single_row_return_side_effect(db_session, commit, report_row): | |||
if report_row.code is None: | |||
return {"success": True, "errors": []} | |||
if report_row.code is "local": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
converted "is" to == which is the proper way to do this comparison
It's in a script though so idk if it really changes anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was already fixed after rebasing 🤷
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #496 +/- ##
==========================================
+ Coverage 97.26% 97.44% +0.18%
==========================================
Files 412 412
Lines 34424 34400 -24
==========================================
+ Hits 33481 33522 +41
+ Misses 943 878 -65
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #496 +/- ##
==========================================
+ Coverage 97.26% 97.44% +0.18%
==========================================
Files 412 412
Lines 34424 34400 -24
==========================================
+ Hits 33481 33522 +41
+ Misses 943 878 -65
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #496 +/- ##
==========================================
+ Coverage 97.26% 97.44% +0.18%
==========================================
Files 412 412
Lines 34424 34400 -24
==========================================
+ Hits 33481 33522 +41
+ Misses 943 878 -65
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Changes have been made to critical files, which contain lines commonly executed in production. Learn more ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #496 +/- ##
==========================================
+ Coverage 97.28% 97.47% +0.18%
==========================================
Files 443 443
Lines 35153 35129 -24
==========================================
+ Hits 34200 34241 +41
+ Misses 953 888 -65
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes This change has been scanned for critical changes. Learn more |
Part of the lint enhancement epic: codecov/engineering-team#1716
This is the "First" Ruff PR for worker, creating the ruff.toml file allowing us to easily configure our ruff rules moving forward, similar to API and Shared. The goal for this PR was to keep the existing "I" and "F401" rules which were already running as well as add a majority of the remaining F rules, and fix as well.
The main rules that were fixed were related to duplicate test names and imports:
Closes codecov/engineering-team#1882
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.